-
Notifications
You must be signed in to change notification settings - Fork 165
Add Token2022 Extensions #267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Mdaeva/tests
…_token_account test
…tension tag strip
test(group_pointer): verify Proxy target's state after clearing group pointer
feat(metadata_pointer): implement extension, proxy, and tests
feat(memo_transfer): full extension + proxy support with tests and token account helper
Extentions Added : - cpi guard - default account state - pausable - scaled ui amount
|
Pending Extensions
|
| /// Return a `TransferHook` from the given bytes (unsafe, unchecked). | ||
| #[inline(always)] | ||
| pub unsafe fn from_bytes_unchecked(bytes: &[u8]) -> &Self { | ||
| &*(bytes[Self::AUTHORITY_START as usize..].as_ptr() as *const TransferHook) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a bit strange to use a fix offset to where the extension will start since it depends on what other extensions are enabled. This is why the original token-2022 code looks up where the extension data is: https://github.com/solana-program/token-2022/blob/main/interface/src/extension/mod.rs#L148-L183
| } | ||
|
|
||
| #[inline(always)] | ||
| pub fn invoke_signed(&self, signers: &[Signer]) -> ProgramResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no signers on this instruction, so we should only have invoke.
| /// Mint Account to initialize. | ||
| pub mint_account: &'a AccountInfo, | ||
| /// Optional authority that can set the transfer hook program id | ||
| pub authority: Option<&'a Pubkey>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use a different lifetime than AccountInfo references. This applies to all Pubkey values.
| pub authority: Option<&'a Pubkey>, | |
| pub authority: Option<&'b Pubkey>, |
| ProgramResult, | ||
| }; | ||
|
|
||
| pub struct InitializeTransferHook<'a> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pub struct InitializeTransferHook<'a> { | |
| pub struct InitializeTransferHook<'a, 'b> { |
| @@ -0,0 +1,215 @@ | |||
| # Pinocchio Interface Test Setup for SPL Token-2022 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cpi-tests should under the token-2022 folder since it is specific to it.
| /// The length of the account with `CpiGuard` extension data | ||
| const LEN: u8 = 171; | ||
| /// The index where CPI guard data starts in the account with `CpiGuard` extension data | ||
| const CPI_GUARD_START: u8 = 170; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't assume a fixed position for the start of the extension data.
|
Thanks for your contribution - it is a good start. Left a few comments that apply to the majority of the extensions. In general:
The PR is also quite big so I suggest breaking it in smaller ones. Start adding the instruction builder for each extension under |
|
Thanks for the detailed feedback, Febo. The point about We will also address the other recommendations accordingly, separating Just to confirm, when we split the extensions into separate PRs, should each extension include its own tests within the same PR, or should we submit the tests in separate PRs afterward? |
Problem
Pinocchio lacks the support for Token2022 extensions.
Solution
Added full Token2022 extension support, including instruction handlers and logic for complete functionality. All tests pass, confirming compatibility with the Token2022 standard.